Skip to content

Fix: Chunksize when opening sharded zarr - #11464

Merged
headtr1ck merged 5 commits into
pydata:mainfrom
taus:open-zarr-shards
Oct 21, 2026
Merged

Fix: Chunksize when opening sharded zarr#11464
headtr1ck merged 5 commits into
pydata:mainfrom
taus:open-zarr-shards

Conversation

@taus

@taus taus commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Use Zarr shards as the preferred chunk size in when opening Zarr datasets. Previously it was necessary to
know the shard size when opening to avoid error saving back to disk.

I think the term chunk is overloaded. In this case, chunk can refer to both Dask chunks and Zarr chunks, which are not necessarily the same. In the case of a shared Zarr archive, i would argue that the sensible choice of Dask chunks is the shard size. When writing to disk it could create race conditions with multiple Dask chunks in the same shard. This is also what caused the original error

Checklist

@github-actions github-actions Bot added topic-backends topic-zarr Related to zarr storage library io labels Jul 21, 2026
@taus
taus marked this pull request as ready for review August 12, 2026 09:02
dcherian
dcherian reviewed Aug 12, 2026
View reviewed changes
Comment thread xarray/tests/test_backends_shards.py Outdated
@taus
taus force-pushed the open-zarr-shards branch 2 times, most recently from d5c6b78 to 4e1d823 Compare August 14, 2026 07:47
dcherian
dcherian reviewed Aug 14, 2026
View reviewed changes
Comment thread xarray/backends/zarr.py Outdated
@taus
taus force-pushed the open-zarr-shards branch from 338e42f to 4e1d823 Compare August 14, 2026 20:48
dcherian
dcherian reviewed Aug 14, 2026
View reviewed changes
Comment thread xarray/backends/zarr.py Outdated
@taus

taus commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

is anything missing for this to be merged?

@taus
taus force-pushed the open-zarr-shards branch from e3f6e6e to e202ecc Compare August 31, 2026 07:01
@headtr1ck headtr1ck mentioned this pull request Oct 12, 2026
Closed
5 tasks done
@taus
taus force-pushed the open-zarr-shards branch 2 times, most recently from 94b5c9b to 22df04a Compare October 15, 2026 11:21
headtr1ck
headtr1ck reviewed Oct 15, 2026
View reviewed changes

@headtr1ck headtr1ck left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to add a whats-new entry.
Couple of smaller fixes.

For me this looks good, maybe someone else should still have a look.

Comment thread xarray/tests/backends/test_zarr.py Outdated
Comment thread xarray/tests/backends/test_zarr.py Outdated
Comment thread xarray/tests/backends/test_zarr.py Outdated
@headtr1ck
headtr1ck requested a review from TomNicholas October 15, 2026 12:03
@taus
taus force-pushed the open-zarr-shards branch from 22df04a to 9ed69b6 Compare October 15, 2026 12:12
@taus
taus force-pushed the open-zarr-shards branch from 9ed69b6 to bad799c Compare October 15, 2026 13:35
@taus
taus force-pushed the open-zarr-shards branch from bad799c to 22cfdc6 Compare October 15, 2026 13:50
@taus

taus commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

@headtr1ck i made the suggested changes. I noticed this issue #11674 and related PR #11680 dealing with the same section. I changed xarray_chunks to outer_chunks, as that seems to be the terminology used.

Maybe @TomNicholas could take a look?

@headtr1ck

headtr1ck commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator

Thanks @taus, this looks good! A few small remaining things:

  • The whats-new entry has a line indented with 3 spaces instead of 2, and a period before (:issue:...).
  • The test is missing @requires_zarr. All the other module-level tests in test_zarr.py have it.
  • dimensions = {f"dim_{i}" ...} is a set, so the order of the dimensions (and which one gets which size) depends on string hashing. A tuple keeps it deterministic.
  • The branch needs main merged in to fix the failing docs build.

I have these fixes ready locally. Could you enable "Allow edits by maintainers" on this PR so I can push them directly?

FYI, this conflicts with #11680 in open_store_variable, but that's easy to resolve. Whichever PR merges second needs to combine _get_zarr_chunks_and_shards() with the outer_chunks line from here.

@taus

taus commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

hey @headtr1ck. I have enable "Allow edits by maintainers"

@headtr1ck headtr1ck added the plan to merge Final call for comments label Oct 20, 2026
@headtr1ck
headtr1ck merged commit 8c079bc into pydata:main Oct 21, 2026
57 of 59 checks passed
@welcome

welcome Bot commented Oct 21, 2026

Copy link
Copy Markdown

Congratulations on completing your first pull request! Welcome to Xarray! We are proud of you, and hope to see you again! celebration gif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dcherian dcherian dcherian approved these changes
@headtr1ck headtr1ck headtr1ck approved these changes
@TomNicholas TomNicholas Awaiting requested review from TomNicholas

Assignees

No one assigned

Labels

io plan to merge Final call for comments topic-backends topic-zarr Related to zarr storage library

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unable to roundtrip sharded zarr

3 participants

@taus @headtr1ck @dcherian